Add support to StartTLS on Quota's mailing by GutoVeronezi · Pull Request #4573 · apache/cloudstack

GutoVeronezi · 2021-01-07T13:45:06Z

Description

On Quota's mailing settings, ACS has a configuration (quota.usage.smtp.useAuth) to inform if it will use secure SMTP authentication when sending emails.
However, this configuration only refers to use SSL or not. Operators haves no option to choose if they want to use StartTLS.

This PR intends to add support to StartTLS on Quota's mailing settings.

It is interesting to highlight that this enhancement would be able to solve issue #2625 (if it was applied in that setting as well); however, to reduce the scope of change, this one focuses only on Quota's mailing configuration.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

It has been tested locally on a test lab.

I had Enabled Quota's plugin on the management server and configured it to use Gmail's SMTP server.

server address: smtp.gmail.com
username: gmail address
password: gmail password
port (TLS): 587

On Quota - All Accounts, I added 10 credits to my user and set 20 as min balance.
I had added an gmail address to Accounts > <account> > Users - Email.
I allowed Less secure app access on my Google account.
I had restarted management server.
And I called quota update API via Cloudmonkey. Without the new option being introduced here, I would see an error in the management server. After setting it, everything is fine, and the alert email is sent.

DaanHoogland

code looks ok, and old behaviour is default, so looks good in that aspect. As a thunderbird user i have the experience that the client can discover what kind of security is used however, can we implement such a thing as well?

RodrigoDLopez

Code LGTM

yadvr · 2021-02-12T07:13:20Z

@blueorangutan package

blueorangutan · 2021-02-12T07:14:14Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-02-12T08:03:30Z

Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2681

DaanHoogland · 2021-02-13T09:46:14Z

@blueorangutan package

blueorangutan · 2021-02-13T09:47:40Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-02-13T11:16:47Z

Packaging result: ✔centos7 ✔centos8 ✖debian. JID-2701

yadvr · 2021-02-19T09:02:25Z

@blueorangutan package

blueorangutan · 2021-02-19T09:04:33Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-02-19T09:04:44Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-02-19T11:49:09Z

Packaging result: ✔centos7 ✖centos8 ✖debian. JID-2756

blueorangutan · 2021-02-19T12:07:43Z

Packaging result: ✔centos7 ✖centos8 ✖debian. JID-2757

yadvr · 2021-03-06T09:20:35Z

@blueorangutan package

blueorangutan · 2021-03-06T09:21:50Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-03-06T09:56:12Z

Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2878

yadvr · 2021-04-05T09:44:30Z

@blueorangutan package

blueorangutan · 2021-04-05T09:45:06Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-04-05T10:40:47Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 325

GutoVeronezi · 2021-04-07T13:22:49Z

@rhtyd @DaanHoogland is there anything else to do?

blueorangutan · 2021-04-09T19:23:35Z

Trillian test result (tid-390)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42094 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4573-t390-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 85 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_deploy_kubernetes_cluster	`Failure`	3609.16	test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster	`Failure`	3602.74	test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	44.66	test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume	`Error`	71.50	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	53.28	test_vm_life_cycle.py

DaanHoogland approved these changes Jan 18, 2021

View reviewed changes

DaanHoogland added this to the 4.16.0.0 milestone Jan 18, 2021

DaanHoogland added complexity:trivial 10 minnutes to a few days at most Severity:Trivial type:enhancement labels Jan 18, 2021

RodrigoDLopez approved these changes Jan 18, 2021

View reviewed changes

yadvr approved these changes Feb 12, 2021

View reviewed changes

GutoVeronezi force-pushed the add-support-to-starttls branch from 43b3d7c to 2109523 Compare February 24, 2021 12:00

Add support to StartTLS on Quota's mailing

e08ef1a

GutoVeronezi force-pushed the add-support-to-starttls branch from 2109523 to e08ef1a Compare February 24, 2021 12:10

yadvr closed this Apr 5, 2021

yadvr reopened this Apr 5, 2021

DaanHoogland merged commit b28d638 into apache:master Apr 13, 2021

yadvr mentioned this pull request Aug 9, 2021

Could not connect to SMTP host using STARTTLS #2625

Closed

Uh oh!

Conversation

GutoVeronezi commented Jan 7, 2021

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

RodrigoDLopez left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Feb 12, 2021

Uh oh!

blueorangutan commented Feb 12, 2021

Uh oh!

blueorangutan commented Feb 12, 2021

Uh oh!

DaanHoogland commented Feb 13, 2021

Uh oh!

blueorangutan commented Feb 13, 2021

Uh oh!

blueorangutan commented Feb 13, 2021

Uh oh!

yadvr commented Feb 19, 2021

Uh oh!

blueorangutan commented Feb 19, 2021

Uh oh!

blueorangutan commented Feb 19, 2021

Uh oh!

blueorangutan commented Feb 19, 2021

Uh oh!

blueorangutan commented Feb 19, 2021

Uh oh!

yadvr commented Mar 6, 2021

Uh oh!

blueorangutan commented Mar 6, 2021

Uh oh!

blueorangutan commented Mar 6, 2021

Uh oh!

yadvr commented Apr 5, 2021

Uh oh!

blueorangutan commented Apr 5, 2021

Uh oh!

blueorangutan commented Apr 5, 2021

Uh oh!

GutoVeronezi commented Apr 7, 2021

Uh oh!

blueorangutan commented Apr 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants